twister: ztest: harness: Fix missed TestCase statuses #80088

golowanow · 2024-10-19T05:32:51Z

Fix a problem of Ztest suite names not taken into account by Twister to identify a TestCase, so in some situations a Ztest test's status was not assigned to the proper TestCase and it remains 'None' whereas the actual status value lost, eventually the resulting total execution counters not correct.

The proposed solution extends Twister test case name for Ztest to include Ztest suite name, so the resulting identifier looks like: <test_scenario_name>.<ztest_suite_name>.<ztest_name>

The above naming scheme now requires ztest_suite_name part to be provided for --sub-test command line option.

Testcase identifiers in twister.json and testplan.json will also include ztest_suite_name component.

The Twister Ztest(Test) Harness is improved to track all state changes known from the test application's log for Ztest suites and test cases, so now it parses log output from a Ztest application more scurpulously. Regular expressions to match log records are extended and optimized to compile them only once and, in some cases, fixed (suite summary).

Twister documentation update.

Fix for C++ Ztest function name 'demangling' - when Ztest application is written in C++ with namespace(s) test function names in ELF are additionally decorated ('mangled').

The issue was observed in these situations (for known in-tree tests affected see #80706):

Ztest application with multiple test suites having same test names.

for example, /scripts/twister -p native_sim -T ./tests/ztest
old:

testing.ztest.summary.shared_unit_test.foo                                  SKIPPED     ztest skip
testing.ztest.summary.shared_unit_test.foo                                  NONE        
...
testing.ztest.error_hook.no_userspace.to_skip0                              PASSED 
testing.ztest.error_hook.no_userspace.to_skip1                              PASSED 
testing.ztest.error_hook.no_userspace.to_skip0                              NONE   
testing.ztest.error_hook.no_userspace.to_skip1                              NONE

new:

testing.ztest.summary.shared_unit_test.suite2.foo                           SKIPPED     ztest skip
testing.ztest.summary.shared_unit_test.suite1.foo                           PASSED      

testing.ztest.error_hook.no_userspace.fail_assume_in_setup.to_skip0         PASSED
testing.ztest.error_hook.no_userspace.fail_assume_in_setup.to_skip1         PASSED
testing.ztest.error_hook.no_userspace.fail_assume_in_before.to_skip0        PASSED
testing.ztest.error_hook.no_userspace.fail_assume_in_before.to_skip1        PASSED

Ztest suite is 'skipped' entirely on execution with all its tests.

for example, ./scripts/twister -p native_sim -T ./tests/drivers/can/api/ -s drivers.can.api
where get_transceiver - was skipped, others - the same situation as example 1
old:

drivers.can.api.get_transceiver                                             NONE
drivers.can.api.invalid_sample_point                                        PASSED
drivers.can.api.set_bitrate_too_high                                        PASSED
drivers.can.api.invalid_sample_point                                        NONE
drivers.can.api.set_bitrate_too_high                                        NONE

new:

drivers.can.api.can_transceiver.get_transceiver                             SKIPPED ztest skip
drivers.can.api.can_classic.invalid_sample_point                            PASSED
drivers.can.api.can_classic.set_bitrate_too_high                            PASSED
drivers.can.api.canfd.invalid_sample_point                                  PASSED
drivers.can.api.canfd.set_bitrate_too_high                                  PASSED

scripts/pylib/twister/twisterlib/harness.py

golowanow · 2024-10-22T13:51:46Z

@arikgreen, do you still have any other objections for your -1 ?

arikgreen · 2024-10-23T08:34:20Z

@arikgreen, do you still have any other objections for your -1 ?

sorry, should be approved

golowanow · 2024-10-23T12:30:06Z

rebase to resolve merge conflicts

nashif · 2024-10-25T15:03:47Z

The proposed solution extends Twister test case name for Ztest to include Ztest suite name, so the resulting identifier looks like: <test_scenario_name>.<ztest_suite_name>.<ztest_name>

ok, this is a rather very intrusive and disruptive change of a very well known problem that we pushed aside for a while now.

The main issue here is that we define suites on different levels, the outcome of transitioning from old ztest to macro based ztest.

We have the testsuite defined at the yaml level and then the suites in ztests, so when you have multiple suites in ztest resuing the same testcases, it faills apart, but that happens in a pair for tests we have right now, so it was easy to ignore.

Some changes in twister were introduced to address this, but never got to the final fix.

I am fine with the fix, the question if it is the right time for it or if we should wait and solicit more discussion and reviews to make sure this does not break someone's downstream.

golowanow · 2024-10-25T15:54:03Z

a rather very intrusive and disruptive change of a very well known problem that we pushed aside for a while now.
.. the question if it is the right time for it or if we should wait and solicit more discussion and reviews to make sure this does not break someone's downstream.

well, although 'very intrusive and disruptive' sounds ominously, it is another old problem, like adjusting to hwmv2 board naming, which should be fixed. Why not to do it "sooner", at 4.0 next major version release ?

golowanow · 2024-10-30T15:37:25Z

Dear reviewers, do you find any objections on this fix for a very well known old problem in Twister, preventing its solution in v.4.0 ?

nashif · 2024-10-31T18:51:30Z

@golowanow please post the list of tests that are showing this behaviour.

golowanow · 2024-10-31T21:53:00Z

please post the list of tests that are showing this behaviour.

The known in-tree tests affected by the problem are now listed in #80706 (will be updated when we find more).

Other changes so far:

rebased to run CI tests with the latest Twister changes.
documentation from doc: twister: Update test naming and application diagram #80103 is now included into this PR as suggested in its review.
C++ test function name 'demanglng' is added.

golowanow · 2024-11-07T18:33:49Z

dear reviewers, ^ up to your attention.

hakehuang · 2024-11-08T09:29:22Z

@golowanow I meet some suspect issues with this pr. if I run

scripts/twister -p frdm_k64f -T samples -T tests

and I get a very long time in compare to the main branch code, and the log shows some warning

WARNING - Unexpected Ztest suite 'test_ipm'
WARNING - Already STARTED 'wdt_basic_test_suite':{'count': 1, 'repeat': 0}
WARNING - Already STARTED 'drivers.watchdog.wdt_basic_test_suite.wdt':{'count': 1}
WARNING - Already STARTED 'wdt_basic_test_suite':{'count': 2, 'repeat': 1}
WARNING - Already STARTED 'drivers.watchdog.wdt_basic_test_suite.wdt':{'count': 2}
WARNING - END  case 'crypto.tinycrypt.tinycrypt_test_cbc_sp_800_38a_encrypt_decrypt' without START detected
WARNING - END  case 'crypto.tinycrypt.tinycrypt_test_ccm_vector_8' without START detected
WARNING - END  case 'crypto.tinycrypt.verify_gf_2_128_double' without START detected
WARNING - END  case 'crypto.tinycrypt.verify_cmac_null_msg' without START detected
WARNING - END  case 'crypto.tinycrypt.verify_cmac_1_block_msg' without START detected
WARNING - END  case 'crypto.tinycrypt.verify_cmac_320_bit_msg' without START detected
WARNING - END  case 'crypto.tinycrypt.verify_cmac_512_bit_msg' without START detected

golowanow · 2024-11-08T10:12:47Z

I meet some suspect issues with this pr. if I run
...
and I get a very long time in compare to the main branch code, and the log shows some warning
WARNING - Unexpected Ztest suite 'test_ipm'
WARNING - Already STARTED 'wdt_basic_test_suite':{'count': 1, 'repeat': 0}
WARNING - Already STARTED 'drivers.watchdog.wdt_basic_test_suite.wdt':{'count': 1}
WARNING - Already STARTED 'wdt_basic_test_suite':{'count': 2, 'repeat': 1}
WARNING - Already STARTED 'drivers.watchdog.wdt_basic_test_suite.wdt':{'count': 2}
WARNING - END case 'crypto.tinycrypt.tinycrypt_test_cbc_sp_800_38a_encrypt_decrypt' without START detected
...

right, these warnings (from 3 tests) are good examples of how this PR allows to spot two kinds of problems not visible before:
1.1) crypto.tinycrypt - the test has a bug and I've created its fix already #80793
1.2) Unexpected Ztest suite 'test_ipm' - is another test's bug #81032 (I've mentioned it yesterday during the Testing WG meeting).
2) drivers.watchdog.wdt_basic_test_suite. - this Ztest is a bit unusual as it restarts its suite recursively so the improved Ztest harness counts that when it sorts out the sequence of START/END events in the log and detects some 'strange' situations.

I left these log records as warnings for now to make such situations visible. The proposed PR addresses the basic level of the Twister/Ztest problems, and the next step will be to improve Twister reporting to show the Suite/Case repetitions correctly.

Fix a problem of Ztest suite names not taken into account by Twister to identify a TestCase, so in some situations a Ztest test's status was not assigned to the proper TestCase and it remains 'None' whereas the actual status value lost, eventually the resulting total execution counters not correct. The issue was observed in these situations: * Ztest application with multiple test suites having same test names. * Ztest suite is 'skipped' entirely on execution with all its tests. The proposed solution extends Twister test case name for Ztest to include Ztest suite name, so the resulting identifier looks like: `<test_scenario_name>.<ztest_suite_name>.<ztest_name>` The above naming scheme now requires ztest_suite_name part to be provided for `--sub-test` command line option. Testcase identifiers in twister.json and testplan.json will also include ztest_suite_name component. The Twister Ztest(Test) Harness is improved to track all state changes known from the test application's log for Ztest suites and test cases, so now it parses log output from a Ztest application more scurpulously. Regular expressions to match log records are extended and optimized to compile them only once and, in some cases, fixed (suite summary). Signed-off-by: Dmitrii Golovanov <[email protected]>

Update Test Application diagram as well as Test Scenario and Test Case naming conventions to make them more clear and aligned to Ztest suite name included as a part of Test Case name. Signed-off-by: Dmitrii Golovanov <[email protected]>

Fix Ztest test function name extraction from ELF symbols for C++ compiled binaries where symbol names need additional 'demangling' to match with corresponding test names. The `c++filt` utility (part of binutils) is called for demangling when it is needed. Twister test suite extension and adjustment. Signed-off-by: Dmitrii Golovanov <[email protected]>

golowanow · 2024-11-13T17:48:52Z

rebased to resolve merge conflicts after #81129

golowanow · 2024-11-16T20:40:16Z

^ @PerMac @nashif

PerMac

Mostly LGTM but have a question I'd like to get answered before accepting/requesting change ("repeated" redundant)?

scripts/pylib/twister/twisterlib/harness.py

nashif · 2024-11-20T13:22:08Z

@PerMac @golowanow before we go forward with this i wanted to discuss enabling --no-detailed-test-id by default, this way we will change how test data is captured and named in one go.

golowanow · 2024-11-20T15:02:45Z

i wanted to discuss enabling --no-detailed-test-id by default, this way we will change how test data is captured and named in one go.

yeah, it might be a next ~~disruptive~~ impactful change, and I can make a draft for it as a separate PR.

This fix has its scope intentionally reduced to focus on the problem it is targeting.

One aspect of detailed-id change to discuss might be to control TestSuite.id prefix presence in testcase.identifier as well.

golowanow · 2024-11-29T14:20:58Z

One aspect of detailed-id change ... to control TestSuite.id prefix presence in testcase.identifier as well.

#82302

golowanow marked this pull request as ready for review October 19, 2024 06:58

zephyrbot added the area: Twister Twister label Oct 19, 2024

zephyrbot requested review from KamilxPaszkiet, LukaszMrugala, PerMac, gchwier, hakehuang and nashif October 19, 2024 06:59

zephyrbot assigned nashif Oct 19, 2024

golowanow mentioned this pull request Oct 20, 2024

doc: twister: Update test naming and application diagram #80103

Closed

arikgreen suggested changes Oct 22, 2024

View reviewed changes

scripts/pylib/twister/twisterlib/harness.py Outdated Show resolved Hide resolved

scripts/pylib/twister/twisterlib/harness.py Outdated Show resolved Hide resolved

scripts/pylib/twister/twisterlib/harness.py Outdated Show resolved Hide resolved

golowanow force-pushed the ztest_twister_20241017 branch from eb6daa8 to e97344b Compare October 22, 2024 08:51

golowanow requested a review from arikgreen October 22, 2024 08:52

arikgreen previously approved these changes Oct 23, 2024

View reviewed changes

golowanow dismissed arikgreen’s stale review via 35ad4d6 October 23, 2024 12:28

golowanow force-pushed the ztest_twister_20241017 branch from e97344b to 35ad4d6 Compare October 23, 2024 12:28

golowanow added the bug The issue is a bug, or the PR is fixing a bug label Oct 25, 2024

golowanow added the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Oct 31, 2024

golowanow force-pushed the ztest_twister_20241017 branch from 35ad4d6 to a343ffe Compare October 31, 2024 21:35

zephyrbot added the area: Testsuite Testsuite label Oct 31, 2024

zephyrbot requested review from aaronemassey, asemjonovs and jeremybettis October 31, 2024 21:35

zephyrbot requested a review from yperess October 31, 2024 21:35

golowanow force-pushed the ztest_twister_20241017 branch from a343ffe to f39a91b Compare October 31, 2024 21:42

golowanow removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Nov 2, 2024

hakehuang previously approved these changes Nov 8, 2024

View reviewed changes

golowanow added 3 commits November 13, 2024 17:44

golowanow dismissed hakehuang’s stale review via 0a563af November 13, 2024 16:46

golowanow force-pushed the ztest_twister_20241017 branch from 3e3126d to 0a563af Compare November 13, 2024 16:46

golowanow requested a review from hakehuang November 13, 2024 17:49

hakehuang approved these changes Nov 15, 2024

View reviewed changes

PerMac reviewed Nov 18, 2024

View reviewed changes

scripts/pylib/twister/twisterlib/harness.py Show resolved Hide resolved

golowanow requested a review from PerMac November 19, 2024 20:55

PerMac approved these changes Nov 20, 2024

View reviewed changes

nashif approved these changes Nov 22, 2024

View reviewed changes

nashif merged commit e11aeca into zephyrproject-rtos:main Nov 22, 2024
31 checks passed

golowanow mentioned this pull request Nov 28, 2024

twister: ztest: short test case names on --no-detailed-test-id #82302

Merged

JordanYates mentioned this pull request Feb 17, 2025

twister: WARNING - TS_START: already STARTED on expected test case reboot #85841

Closed

twister: ztest: harness: Fix missed TestCase statuses #80088

twister: ztest: harness: Fix missed TestCase statuses #80088

Uh oh!

Conversation

golowanow commented Oct 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The issue was observed in these situations (for known in-tree tests affected see #80706):

Uh oh!

Uh oh!

Uh oh!

Uh oh!

golowanow commented Oct 22, 2024

Uh oh!

arikgreen commented Oct 23, 2024

Uh oh!

golowanow commented Oct 23, 2024

Uh oh!

nashif commented Oct 25, 2024

Uh oh!

golowanow commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

golowanow commented Oct 30, 2024

Uh oh!

nashif commented Oct 31, 2024

Uh oh!

golowanow commented Oct 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

golowanow commented Nov 7, 2024

Uh oh!

hakehuang commented Nov 8, 2024

Uh oh!

golowanow commented Nov 8, 2024

Uh oh!

golowanow commented Nov 13, 2024

Uh oh!

golowanow commented Nov 16, 2024

Uh oh!

PerMac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nashif commented Nov 20, 2024

Uh oh!

golowanow commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

golowanow commented Nov 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

golowanow commented Oct 19, 2024 •

edited

Loading

golowanow commented Oct 25, 2024 •

edited

Loading

golowanow commented Oct 31, 2024 •

edited

Loading

golowanow commented Nov 20, 2024 •

edited

Loading